Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

made builder traits internal #1376

Merged
merged 6 commits into from
Sep 9, 2024
Merged

made builder traits internal #1376

merged 6 commits into from
Sep 9, 2024

Conversation

milyin
Copy link
Contributor

@milyin milyin commented Sep 8, 2024

@wyfo proposed to remove the traits like QoSBuiderTrait, SampleBuilderTrait, etc as these traits have zero value for end user and only adds additional burden of importing these traits and having troubles with auto completion when traits are not imported.
These arguments are valid, but on the other hand these traits were added to provide strict and automatic guarantees of API uniformity. Removing them adds risks that some pieces of API becomes forgotten or implenented differently for different entities.
This PR provides a solution: the macro zenoh_macros::internal_trait which automatically generates corresponding structure impl functions from the trait

Copy link

github-actions bot commented Sep 8, 2024

PR missing one of the required labels: {'enhancement', 'bug', 'documentation', 'dependencies', 'internal', 'breaking-change', 'new feature'}

Copy link

github-actions bot commented Sep 8, 2024

PR missing one of the required labels: {'enhancement', 'dependencies', 'breaking-change', 'documentation', 'new feature', 'bug', 'internal'}

Copy link

github-actions bot commented Sep 8, 2024

PR missing one of the required labels: {'new feature', 'internal', 'breaking-change', 'bug', 'enhancement', 'documentation', 'dependencies'}

@milyin milyin added enhancement Existing things could work better breaking-change Indicates that the issue implies a breaking change (be it at compile time or at runtime) labels Sep 8, 2024
@milyin milyin requested review from wyfo and Mallets and removed request for wyfo September 8, 2024 21:17
@Mallets Mallets merged commit 55d5395 into main Sep 9, 2024
25 of 26 checks passed
@Mallets Mallets deleted the scaffolding2 branch September 9, 2024 09:51
Mallets added a commit that referenced this pull request Sep 9, 2024
* Fix bug with QueryTarget ALL_COMPLETE in clients and peers (#1358)

* Fix bug with QueryTarget ALL_COMPLETE in clients and peers

* Fix BEST_MATCHING queryable selection

* Properly fix Query targeting in non writer side filtering situations

* Improve fix

* Update zenoh/src/net/routing/hat/linkstate_peer/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Update zenoh/src/net/routing/hat/p2p_peer/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Update zenoh/src/net/routing/hat/router/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Update zenoh/src/net/routing/hat/client/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Remove non used ordered-float dependency

---------

Co-authored-by: Luca Cominardi <[email protected]>
Co-authored-by: Joseph Perez <[email protected]>

* fix: publisher should not be clonable (#1370)

* made builder traits internal (#1376)

* scaffolding macro added

* builder traits made internal

* doc corrected

* cargo fmt

* typo fix

* typo fix

* Fix bugs querying liveliness tokens (#1374)

* Fix bug in liveliness get in client

* Fix bug treating token interests replies from routers in peers

* Peers propagate current token interests to remote peers with unfinalize initial declarations push

* Don't register current interests declaration replies

* Add comments

* Add comments

* Add comments

---------

Co-authored-by: OlivierHecart <[email protected]>
Co-authored-by: Luca Cominardi <[email protected]>
Co-authored-by: Michael Ilyin <[email protected]>
Mallets added a commit that referenced this pull request Sep 12, 2024
* feat!: bind callback subscriber/queryable to session lifetime

To determine if the entity is callback-only, the only elegant way I've found is the rule "handler is ZST means callback-only". Unless users starts writing fancy implementations, it should be correct 100% of the time.

Session entities now uses weak references, except publishers because it would impact performances. Weak references also solves the issue of mass undeclarations before closing the session (when the session is an `Arc`), except for publishers.

`Undeclarable` trait has been refactored a little bit to better match its use in the code.

* fix: fix example

* fix: fix example

* fix: add missing comment about ZST trick

* Update zenoh/src/api/key_expr.rs

Co-authored-by: Luca Cominardi <[email protected]>

* fix: formatting

* fix: don't use `Weak` when undeclared on drop

* feat!: make session an arc-like object

The refactoring is quite deep, so this is the first (dirty) iteration
which passes the tests.

* fix: use weak everywhere!

* fix: fix doc

* feat: use pseudo-weak session with the same perf than arc

* fix: fix resource cleanup

* Fix typo

* fix: align `MatchingListener` undeclaration on drop behavior

* refactor: add comments about `WeakSession`

* refactor: add comment for `Session` and `Session::close` (#1369)

* refactor: add comment for `Session` and `Session::close`

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* fix: don't run `Session::close` example

* fix: fix `Session` example

* fix: fix `Session` doc

---------

Co-authored-by: Luca Cominardi <[email protected]>

* feat: use builder method instead of handler type for undeclaration on drop (#1377)

* refactor: add comment for `Session` and `Session::close`

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* fix: don't run `Session::close` example

* fix: fix `Session` example

* fix: fix `Session` doc

* feat: use builder method instead of handler type for undeclaration on drop

---------

Co-authored-by: Luca Cominardi <[email protected]>

* chore: merge main into dev/arcsession (#1378)

* Fix bug with QueryTarget ALL_COMPLETE in clients and peers (#1358)

* Fix bug with QueryTarget ALL_COMPLETE in clients and peers

* Fix BEST_MATCHING queryable selection

* Properly fix Query targeting in non writer side filtering situations

* Improve fix

* Update zenoh/src/net/routing/hat/linkstate_peer/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Update zenoh/src/net/routing/hat/p2p_peer/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Update zenoh/src/net/routing/hat/router/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Update zenoh/src/net/routing/hat/client/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Remove non used ordered-float dependency

---------

Co-authored-by: Luca Cominardi <[email protected]>
Co-authored-by: Joseph Perez <[email protected]>

* fix: publisher should not be clonable (#1370)

* made builder traits internal (#1376)

* scaffolding macro added

* builder traits made internal

* doc corrected

* cargo fmt

* typo fix

* typo fix

* Fix bugs querying liveliness tokens (#1374)

* Fix bug in liveliness get in client

* Fix bug treating token interests replies from routers in peers

* Peers propagate current token interests to remote peers with unfinalize initial declarations push

* Don't register current interests declaration replies

* Add comments

* Add comments

* Add comments

---------

Co-authored-by: OlivierHecart <[email protected]>
Co-authored-by: Luca Cominardi <[email protected]>
Co-authored-by: Michael Ilyin <[email protected]>

* Revert "chore: merge main into dev/arcsession (#1378)" (#1379)

This reverts commit f3f5403.

* fix: various fixes

* fix: various fixes (2)

* chore: merge branch 'main' into 'dev/arcsession' (#1384)

* feat(zenoh_id): exposing into slice & try from slice

Allowing users to create a ZenohId from a slice, using TryFrom, and also
allowing users to convert a ZenohId into a [u8; 16].

* Add LivlinessSubscriber history option (#1355)

* Close #1357

* feat(zenoh_id): replacing from slice with  function

---------

Co-authored-by: Darius Maitia <[email protected]>
Co-authored-by: OlivierHecart <[email protected]>
Co-authored-by: Luca Cominardi <[email protected]>

* refactor: use `IntoHandler` associated const for undeclaration on drop

* refactor: remove prelude

* fix: fix config example

* fix: fix zenoh-ext examples

* fix: fix plugins examples

* fix: fix merge

* Fix shm test

* Fix clippy warnings

* refactor: use builder method for undeclaration on drop

* fix: remove useless lifetimes

* fix: add missing unstable flag

* feat: add tuple implementation for IntoHandler

---------

Co-authored-by: Joseph Perez <[email protected]>
Co-authored-by: OlivierHecart <[email protected]>
Co-authored-by: Michael Ilyin <[email protected]>
Co-authored-by: Darius Maitia <[email protected]>
fuzzypixelz pushed a commit that referenced this pull request Sep 23, 2024
* scaffolding macro added

* builder traits made internal

* doc corrected

* cargo fmt

* typo fix

* typo fix
fuzzypixelz pushed a commit that referenced this pull request Sep 23, 2024
* feat!: bind callback subscriber/queryable to session lifetime

To determine if the entity is callback-only, the only elegant way I've found is the rule "handler is ZST means callback-only". Unless users starts writing fancy implementations, it should be correct 100% of the time.

Session entities now uses weak references, except publishers because it would impact performances. Weak references also solves the issue of mass undeclarations before closing the session (when the session is an `Arc`), except for publishers.

`Undeclarable` trait has been refactored a little bit to better match its use in the code.

* fix: fix example

* fix: fix example

* fix: add missing comment about ZST trick

* Update zenoh/src/api/key_expr.rs

Co-authored-by: Luca Cominardi <[email protected]>

* fix: formatting

* fix: don't use `Weak` when undeclared on drop

* feat!: make session an arc-like object

The refactoring is quite deep, so this is the first (dirty) iteration
which passes the tests.

* fix: use weak everywhere!

* fix: fix doc

* feat: use pseudo-weak session with the same perf than arc

* fix: fix resource cleanup

* Fix typo

* fix: align `MatchingListener` undeclaration on drop behavior

* refactor: add comments about `WeakSession`

* refactor: add comment for `Session` and `Session::close` (#1369)

* refactor: add comment for `Session` and `Session::close`

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* fix: don't run `Session::close` example

* fix: fix `Session` example

* fix: fix `Session` doc

---------

Co-authored-by: Luca Cominardi <[email protected]>

* feat: use builder method instead of handler type for undeclaration on drop (#1377)

* refactor: add comment for `Session` and `Session::close`

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* fix: don't run `Session::close` example

* fix: fix `Session` example

* fix: fix `Session` doc

* feat: use builder method instead of handler type for undeclaration on drop

---------

Co-authored-by: Luca Cominardi <[email protected]>

* chore: merge main into dev/arcsession (#1378)

* Fix bug with QueryTarget ALL_COMPLETE in clients and peers (#1358)

* Fix bug with QueryTarget ALL_COMPLETE in clients and peers

* Fix BEST_MATCHING queryable selection

* Properly fix Query targeting in non writer side filtering situations

* Improve fix

* Update zenoh/src/net/routing/hat/linkstate_peer/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Update zenoh/src/net/routing/hat/p2p_peer/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Update zenoh/src/net/routing/hat/router/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Update zenoh/src/net/routing/hat/client/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Remove non used ordered-float dependency

---------

Co-authored-by: Luca Cominardi <[email protected]>
Co-authored-by: Joseph Perez <[email protected]>

* fix: publisher should not be clonable (#1370)

* made builder traits internal (#1376)

* scaffolding macro added

* builder traits made internal

* doc corrected

* cargo fmt

* typo fix

* typo fix

* Fix bugs querying liveliness tokens (#1374)

* Fix bug in liveliness get in client

* Fix bug treating token interests replies from routers in peers

* Peers propagate current token interests to remote peers with unfinalize initial declarations push

* Don't register current interests declaration replies

* Add comments

* Add comments

* Add comments

---------

Co-authored-by: OlivierHecart <[email protected]>
Co-authored-by: Luca Cominardi <[email protected]>
Co-authored-by: Michael Ilyin <[email protected]>

* Revert "chore: merge main into dev/arcsession (#1378)" (#1379)

This reverts commit f3f5403.

* fix: various fixes

* fix: various fixes (2)

* chore: merge branch 'main' into 'dev/arcsession' (#1384)

* feat(zenoh_id): exposing into slice & try from slice

Allowing users to create a ZenohId from a slice, using TryFrom, and also
allowing users to convert a ZenohId into a [u8; 16].

* Add LivlinessSubscriber history option (#1355)

* Close #1357

* feat(zenoh_id): replacing from slice with  function

---------

Co-authored-by: Darius Maitia <[email protected]>
Co-authored-by: OlivierHecart <[email protected]>
Co-authored-by: Luca Cominardi <[email protected]>

* refactor: use `IntoHandler` associated const for undeclaration on drop

* refactor: remove prelude

* fix: fix config example

* fix: fix zenoh-ext examples

* fix: fix plugins examples

* fix: fix merge

* Fix shm test

* Fix clippy warnings

* refactor: use builder method for undeclaration on drop

* fix: remove useless lifetimes

* fix: add missing unstable flag

* feat: add tuple implementation for IntoHandler

---------

Co-authored-by: Joseph Perez <[email protected]>
Co-authored-by: OlivierHecart <[email protected]>
Co-authored-by: Michael Ilyin <[email protected]>
Co-authored-by: Darius Maitia <[email protected]>
fuzzypixelz pushed a commit that referenced this pull request Sep 23, 2024
* scaffolding macro added

* builder traits made internal

* doc corrected

* cargo fmt

* typo fix

* typo fix
fuzzypixelz pushed a commit that referenced this pull request Sep 23, 2024
* feat!: bind callback subscriber/queryable to session lifetime

To determine if the entity is callback-only, the only elegant way I've found is the rule "handler is ZST means callback-only". Unless users starts writing fancy implementations, it should be correct 100% of the time.

Session entities now uses weak references, except publishers because it would impact performances. Weak references also solves the issue of mass undeclarations before closing the session (when the session is an `Arc`), except for publishers.

`Undeclarable` trait has been refactored a little bit to better match its use in the code.

* fix: fix example

* fix: fix example

* fix: add missing comment about ZST trick

* Update zenoh/src/api/key_expr.rs

Co-authored-by: Luca Cominardi <[email protected]>

* fix: formatting

* fix: don't use `Weak` when undeclared on drop

* feat!: make session an arc-like object

The refactoring is quite deep, so this is the first (dirty) iteration
which passes the tests.

* fix: use weak everywhere!

* fix: fix doc

* feat: use pseudo-weak session with the same perf than arc

* fix: fix resource cleanup

* Fix typo

* fix: align `MatchingListener` undeclaration on drop behavior

* refactor: add comments about `WeakSession`

* refactor: add comment for `Session` and `Session::close` (#1369)

* refactor: add comment for `Session` and `Session::close`

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* fix: don't run `Session::close` example

* fix: fix `Session` example

* fix: fix `Session` doc

---------

Co-authored-by: Luca Cominardi <[email protected]>

* feat: use builder method instead of handler type for undeclaration on drop (#1377)

* refactor: add comment for `Session` and `Session::close`

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* Update zenoh/src/api/session.rs

Co-authored-by: Luca Cominardi <[email protected]>

* fix: don't run `Session::close` example

* fix: fix `Session` example

* fix: fix `Session` doc

* feat: use builder method instead of handler type for undeclaration on drop

---------

Co-authored-by: Luca Cominardi <[email protected]>

* chore: merge main into dev/arcsession (#1378)

* Fix bug with QueryTarget ALL_COMPLETE in clients and peers (#1358)

* Fix bug with QueryTarget ALL_COMPLETE in clients and peers

* Fix BEST_MATCHING queryable selection

* Properly fix Query targeting in non writer side filtering situations

* Improve fix

* Update zenoh/src/net/routing/hat/linkstate_peer/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Update zenoh/src/net/routing/hat/p2p_peer/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Update zenoh/src/net/routing/hat/router/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Update zenoh/src/net/routing/hat/client/queries.rs

Co-authored-by: Joseph Perez <[email protected]>

* Remove non used ordered-float dependency

---------

Co-authored-by: Luca Cominardi <[email protected]>
Co-authored-by: Joseph Perez <[email protected]>

* fix: publisher should not be clonable (#1370)

* made builder traits internal (#1376)

* scaffolding macro added

* builder traits made internal

* doc corrected

* cargo fmt

* typo fix

* typo fix

* Fix bugs querying liveliness tokens (#1374)

* Fix bug in liveliness get in client

* Fix bug treating token interests replies from routers in peers

* Peers propagate current token interests to remote peers with unfinalize initial declarations push

* Don't register current interests declaration replies

* Add comments

* Add comments

* Add comments

---------

Co-authored-by: OlivierHecart <[email protected]>
Co-authored-by: Luca Cominardi <[email protected]>
Co-authored-by: Michael Ilyin <[email protected]>

* Revert "chore: merge main into dev/arcsession (#1378)" (#1379)

This reverts commit f3f5403.

* fix: various fixes

* fix: various fixes (2)

* chore: merge branch 'main' into 'dev/arcsession' (#1384)

* feat(zenoh_id): exposing into slice & try from slice

Allowing users to create a ZenohId from a slice, using TryFrom, and also
allowing users to convert a ZenohId into a [u8; 16].

* Add LivlinessSubscriber history option (#1355)

* Close #1357

* feat(zenoh_id): replacing from slice with  function

---------

Co-authored-by: Darius Maitia <[email protected]>
Co-authored-by: OlivierHecart <[email protected]>
Co-authored-by: Luca Cominardi <[email protected]>

* refactor: use `IntoHandler` associated const for undeclaration on drop

* refactor: remove prelude

* fix: fix config example

* fix: fix zenoh-ext examples

* fix: fix plugins examples

* fix: fix merge

* Fix shm test

* Fix clippy warnings

* refactor: use builder method for undeclaration on drop

* fix: remove useless lifetimes

* fix: add missing unstable flag

* feat: add tuple implementation for IntoHandler

---------

Co-authored-by: Joseph Perez <[email protected]>
Co-authored-by: OlivierHecart <[email protected]>
Co-authored-by: Michael Ilyin <[email protected]>
Co-authored-by: Darius Maitia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Indicates that the issue implies a breaking change (be it at compile time or at runtime) enhancement Existing things could work better
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants